-
Notifications
You must be signed in to change notification settings - Fork 1.2k
build: Install libraries in an arch
sub-folder
#5257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
build: Install libraries in an arch
sub-folder
#5257
Conversation
@swift-ci please test |
ef23f75
to
ea40b13
Compare
@swift-ci please test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not entirely sure I'm following what this change is intended to do - could you elaborate on the background/context a bit more? My current understanding is that when we ship the toolchain we ship a single swift module with multiple architectures within it. Do I have that right? And if so, is this intending to change how that is done but only on some platforms where SwiftFoundation_INSTALL_ARCH_SUBDIR
is set?
This is the proper installation scheme for Swift libraries and prevents having to manually copy them in `build.ps1`.
* Update the C and C++ libraries to also install the arch sub-directory. * Simplify the logic to gather the arch and triple data.
ea40b13
to
81c5202
Compare
@jmschonfeld PTAL, my apologies it took me a while to get back to this.
The Windows toolchain layout expects that the native libraries are installed in an architecture sub-directory. There is a manual step in the build script for Windows to copy the libraries in the right folder that warns about these in this manner:
This PR aims to fix the problem by installing the libraries in the right directory with this new configuration option. |
Thanks for the explanation. Overall this seems alright then, but:
If this location is the correct location for Windows, should we just gate this based on whether we're building for Windows rather than a custom config option? In general if there isn't a need for toggling this on/off, it might be better to just decide what the right value is to reduce the number of configurations Foundation's build is forked into. Also, does this change need to land in-sync with any other changes (either to the build script, or doing the same thing to other Foundation libraries in swift-foundation and swift-foundation-icu)? Would also be great to have @compnerd and @etcwilde take a look at this since they're more familiar with our build setups than I am |
When I say "Windows", I mean the toolchain that we install on Windows, which includes Windows and Android targets for this project. While we could rely on the host platform OS, in the future, we'd also like to support building cross-platform so it does have to be a separate option.
It doesn't need to be in sync, I can land the configuration change in |
Should we just conditionalize this based on the platform for which we're building then (i.e. if building for Windows or Android regardless of host OS then we should do this), or are there scenarios in which you'd want to build for Windows or Android and not do this? I think my line of thinking is that if this option should always be set for certain environments, we should just put that check in Foundation instead of hoisting that check somewhere else that we don't control and is unlikely to be in sync with CI setups. But if there's value in the option because it needs to be conditionally enabled based on some other factor that only some other factor can provide that could merit this. We just like to minimize the combinatorial number of ways Foundation can be built to ensure we have high test coverage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I think this is better. Just the question about whether or not we want to conditionalize the entire nesting bit or not.
To speak to @jmschonfeld's question:
This is more about how we configure the package for installation than how Foundation itself is built. SwiftPM doesn't have an install story, so I don't think it can verify that rpaths and search paths are set up correctly through the testing today.
Unless Foundation is hard-coding these paths in the code itself, it is useful to allow vendors to specify how they want to lay out the installation. Right now the only vendor is Swift.org, but we are interested in improving the install story by distro package manager, which will require giving the vendor flexibility to install the library according to their filesystem hierarchy design. Swift.org has these directories so that you can merge multiple SDKs into one, though we don't really make use of it in the Linux SDKs. A distro package manager has no such need because it knows exactly what OS and architecture you're on, so it only needs to install the one runtime and doesn't need the extra nesting at all. Some distros have multi-arch support, but each has its own layout for handling that, which does not align with what we are doing.
@@ -148,7 +148,7 @@ install(DIRECTORY | |||
|
|||
if(NOT BUILD_SHARED_LIBS) | |||
install(TARGETS CoreFoundation | |||
ARCHIVE DESTINATION lib/swift_static/${SWIFT_SYSTEM_NAME} | |||
LIBRARY DESTINATION lib/swift_static/${SWIFT_SYSTEM_NAME} | |||
ARCHIVE DESTINATION lib/swift_static/${SwiftFoundation_PLATFORM}$<$<BOOL:${SwiftFoundation_INSTALL_ARCH_SUBDIR}>:/${SwiftFoundation_ARCH}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the architecture be the only optional component? In the standard library, we made both the platform and arch portion configurable with SwiftCore_INSTALL_NESTED_SUBDIR
.
option(SwiftCore_INSTALL_NESTED_SUBDIR "Install libraries under a platform and architecture subdirectory" ON)
set(SwiftCore_INSTALL_LIBDIR "${CMAKE_INSTALL_LIBDIR}/swift$<$<NOT:$<BOOL:${BUILD_SHARED_LIBS}>>:_static>$<$<BOOL:${SwiftCore_INSTALL_NESTED_SUBDIR}>:/${SwiftCore_PLATFORM_SUBDIR}/${SwiftCore_ARCH_SUBDIR}>")
set(SwiftCore_INSTALL_SWIFTMODULEDIR "${CMAKE_INSTALL_LIBDIR}/swift$<$<NOT:$<BOOL:${BUILD_SHARED_LIBS}>>:_static>$<$<BOOL:${SwiftCore_INSTALL_NESTED_SUBDIR}>:/${SwiftCore_PLATFORM_SUBDIR}>")
This is the proper installation scheme for Swift libraries on Windows and prevents having to manually copy them in
build.ps1
.